Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Krige, Field and Covmodel update #67

Merged
merged 87 commits into from
Feb 8, 2020
Merged

Krige, Field and Covmodel update #67

merged 87 commits into from
Feb 8, 2020

Conversation

MuellerSeb
Copy link
Member

@MuellerSeb MuellerSeb commented Jan 23, 2020 •
edited
Loading

With this PR a revised kriging submodule is proposed. Some minor additions to Covmodel and Field were also made in the course of this.

Kriging changes

Now the following kriging methods are provided, based on a new kriging base class:

  • Simple
  • Ordinary
  • Universal
  • External Drift Kriging
  • Detrended Kriging

All methods can make use of a callable trend like in detrended kriging (which is simply a shortcut for simple kriging with a demanded trend).

Field changes

  • added pre_pos method to Field class to pre-process the position tuple
  • conditioned SRF now check if the field is "scalar"

CovModel changes

  • in 3D the input of anisotropy is now treated slightly different:
    • single given anisotropy value [e] is converted to [1, e] (it was [e, e] before)
    • two given length-scales [l_1, l_2] are converted to [l_1, l_2, l_2] (it was [l_1, l_2, l_1] before)
  • the TPLGaussian and TPLExponential now have analytical spectra
  • added property is_isotropic to CovModel

Still missing

  • tests
  • new style examples

@MuellerSeb MuellerSeb added enhancement New feature or request Performance Performance related stuff. Refactoring Code-Refactoring needed here labels Jan 23, 2020
@MuellerSeb MuellerSeb added this to the 1.2 milestone Jan 23, 2020
@MuellerSeb MuellerSeb self-assigned this Jan 23, 2020
@MuellerSeb
Copy link
Member Author

MuellerSeb commented Jan 23, 2020 •
edited
Loading

Somehow I polluted the history by rebasing locally.

@LSchueler : I don't if squashing would be a good idea since there are a lot of different changes. Sorry for mixing up stuff. Do you have an idea on how to pick out the relevant commits?

@MuellerSeb MuellerSeb changed the title Krige update Krige, Field and Covmodel update Jan 23, 2020
@LSchueler
Copy link
Member

I'm your Mr. Wolf for git problems right?! :-)

I'll see what I can do.

@MuellerSeb MuellerSeb assigned LSchueler and unassigned MuellerSeb Jan 24, 2020
@MuellerSeb MuellerSeb marked this pull request as ready for review January 24, 2020 10:03
@MuellerSeb MuellerSeb requested a review from LSchueler January 24, 2020 10:03
@LSchueler
Copy link
Member

Did I get it correctly, that all commits from b13845b to 87bb37d can be deleted? And the merge c14ab5b too?

@MuellerSeb
Copy link
Member Author

@LSchueler : yepp.

@MuellerSeb
Copy link
Member Author

I'd say it's ready for review. @LSchueler

@MuellerSeb MuellerSeb mentioned this pull request Jan 27, 2020
Open
10 tasks
@MuellerSeb
Copy link
Member Author

Resolved merge-conflicts.

MuellerSeb
MuellerSeb commented Jan 27, 2020
View reviewed changes
Copy link
Member Author

@MuellerSeb MuellerSeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I am happy now.

LSchueler
LSchueler requested changes Feb 7, 2020
View reviewed changes
Copy link
Member

@LSchueler LSchueler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

Just some very minor things as comments in the code and sometimes you use the variable ax_lens and sometimes axis_lens. We should stick to only one.

And I pushed some typo fixes.

gstools/field/base.py Show resolved Hide resolved
@MuellerSeb
Copy link
Member Author

Great work!

Just some very minor things as comments in the code and sometimes you use the variable ax_lens and sometimes axis_lens. We should stick to only one.

And I pushed some typo fixes.

Thanks for the typo hunting! I renamed every occurrence axis_lens so its unique.

@MuellerSeb MuellerSeb merged commit c7e526d into develop Feb 8, 2020
@MuellerSeb MuellerSeb deleted the krige_update branch February 8, 2020 15:42
@MuellerSeb MuellerSeb mentioned this pull request Mar 20, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LSchueler LSchueler LSchueler requested changes

Assignees

@LSchueler LSchueler

Labels
enhancement New feature or request Performance Performance related stuff. Refactoring Code-Refactoring needed here
Projects
None yet
Milestone
1.2
Development

Successfully merging this pull request may close these issues.
2 participants
@MuellerSeb @LSchueler